fix(forge-gemm): MoE runtime key, E2E faults, warm-replay nogit, model path - #1238

Stop forge GEMM E2E from recording boot/measurement failures as REVERT
with 0% gain; route them through e2e_results.faults instead. Teach the
tuned-config coverage check to match MoE dispatch tuples from server.log
against fmoe CSV rows rather than treating every MoE table as empty.

Co-authored-by: Cursor <cursoragent@cursor.com>

Give each forge GEMM tuner up to _MAX_INTEGRATE_FAULT_ATTEMPTS integrate
attempts before recording an integrate_fault, matching the kernel_opt
fault retry semantics. Repair the stopped-run unit test that was merged
into the bench-fault case during the prior edit.

Co-authored-by: Cursor <cursoragent@cursor.com>

Warm-replay patches now fall back to the shared nogit patch applier for
non-git install trees and unborn git repos, so enablement overlays reach
runtime instead of failing with missing_git_head.

Co-authored-by: Cursor <cursoragent@cursor.com>

The gemm_tuning breakdown now prefers e2e_gain_pct when present so KEEP
runs report the validated end-to-end delta rather than the micro benchmark
speedup alone.

Co-authored-by: Cursor <cursoragent@cursor.com>

Forge KEEP now copies fmoe and dense tuned tables into the serving aiter
config tree and snapshots them together, instead of hardcoding only the
a8w8 blockscale env key.

Co-authored-by: Cursor <cursoragent@cursor.com>

…grate

Add shared session model-path precedence with HL_MODEL_BASE and HF-cache
fallback, re-export MODEL_PATH at CLI startup, and pass the resolved path
into forge GEMM E2E integrate calls.

Co-authored-by: Cursor <cursoragent@cursor.com>

Baseline, explore, integrate, sweep, and conc_sweep now share the same
params -> MODEL_PATH -> state precedence and serving-path normalization.

Co-authored-by: Cursor <cursoragent@cursor.com>

Brings the branch base forward 45 commits. Picks up the docs header-button
and project-metadata fixes (4be7e9d, f17222c) that the sphinx html job
needs -- without them sphinx_book_theme's add_source_buttons raised
"cannot unpack non-iterable NoneType" and the build failed on this branch
while passing on branches with a newer base.

All five overlapping files (cli/__init__.py, baseline.py, request_handlers.py,
phases/kernel.py, test_coordinator_gemm_promote_units.py) merged without
conflict.

Verified: the change-related suites report 5 failed / 183 passed / 11 skipped.
All 5 failures are pre-existing and reproduce identically on both the
pre-merge branch head (3 failed / 62 passed on the two affected files) and on
pristine origin/main (3 failed / 47 passed on the same two files); the
remaining 2 are the Windows-only missing-patch-CLI cases. No regression.

Windows cannot collect test_coordinator_gemm_promote_units.py after this
merge because main's recipe_kb/local_store.py imports fcntl, which is
POSIX-only; that suite is covered by CI on Linux.

Co-authored-by: Cursor <cursoragent@cursor.com>

Brings the base forward 49 commits, onto 4731f47. That head is the #1230
merge, which carries the docs/conf.py repository-key fix the sphinx html job
needs -- the job failed on this branch only because sphinx-book-theme's
get_repo_parts() returned None and its caller unpacked it unconditionally.

One conflict, in test_geak_breakdown_unit.py: both sides appended tests at the
end of the file (26 lines here, 380 on main). Pure append collision, no shared
semantics; both sets are kept.

Verified against pristine 4731f47 on the same seven suites: main reports
7 failed / 168 passed / 7 skipped, this branch 5 failed / 196 passed /
11 skipped. The failures here are a strict subset of main's -- the two
warm-patch cases main fails are fixed on this branch -- so no regression.
Remaining failures are Windows-only: no patch CLI, and main's cli/kb.py
imports fcntl.

Co-authored-by: Cursor <cursoragent@cursor.com>

Four guards, all on paths this branch introduced or newly made load-bearing.
Every one turns a failure to reach a verdict into "undetermined" instead of
into a verdict, which is the same conflation the branch exists to remove: a
measurement that never happened must not read as a measurement of zero.

* E2E validation is now guarded as a whole. Both entrypoints wrap only the
  tuning call, so an exception from the validation that follows -- server
  restarts, log parsing, CSV merges -- took the KERNEL phase down over a
  candidate that had simply gone unmeasured. It now records an
  ``e2e_validation_exception`` fault and lets the phase continue.

* The tuned-config coverage report no longer answers when it cannot read its
  own artifact. An unreadable or schema-shifted CSV yields no keys, which the
  report scored as 0% coverage -- and 0% blocks a KEEP. That let a corrupt
  file revert a candidate whose throughput genuinely improved. An empty key
  set now returns "undetermined", matching how an empty request set was
  already handled. A readable CSV whose keys miss still reports 0%, so the
  real check is unweakened.

* That report also gets the blanket guard its sibling ``_gemm_apply_verdict``
  already carries, plus a safe mtime helper: sorting logs by mtime races the
  round still writing them, and an ``exists()`` check does not close the
  window.

* Writing the MoE untuned CSV tolerates a full disk. ``mkdir``/``write_text``
  were unguarded, so an OSError escaped into the tuning run; the dense tuners
  take their shapes from elsewhere and can still deliver something.

Forge's model-path probe now matches bootstrap's. Bootstrap walks
HL_MODEL_BASE and the hub cache and falls back to the raw string, so a repo id
the running server resolved fine was rejected here by a hub-cache-only probe.
Unresolvable input is now ``skipped`` rather than ``failed``: forge never ran,
so it has no verdict, and reporting one spends a REVERT on an experiment that
never started.

Tests: the coverage and E2E guards are pinned by mutation (reverting either
one fails 3 and 2 cases respectively). Affected suites hold at 5 pre-existing
Windows-only failures against pristine origin/main, with 12 added cases.

Co-authored-by: Cursor <cursoragent@cursor.com>

aiter merges every ``model_configs/*{table}*.csv`` it can glob whenever the
matching env var is unset, which is the ordinary case for a plain server start.
The durable copy landed directly in that directory, and it landed during the
micro phase -- before E2E has ruled on the candidate. So a candidate E2E went
on to reject still reached every later server: the verdict read REVERT while
the table was silently in effect, poisoning the baseline that subsequent gains
are measured against, and persisting across sessions because it lives in the
installed package rather than the session.

The scan does not discriminate by model either. A real V4-Flash run merged
dsv3's table, so one model's tuning reaches another's serving.

Replay does not need the scan: it restores the env var explicitly from
``e2e_results.kept[].env_var`` and defers a GEMM column that has no env at all
(``prelude._warm_kernel_extra_envs``). The auto-merge was a side effect, not the
mechanism durability relies on -- so moving the copy one level down costs
nothing and ends the leak. The glob is not recursive.

Verified against the installed aiter on gfx950: a probe in the subdirectory is
absent from both aiter's own selection expression and its merge banner, while
the same filename directly in ``model_configs/`` is picked up by both -- so the
scan is live and the subdirectory is genuinely out of its reach.

Co-authored-by: Cursor <cursoragent@cursor.com>

The session already knows its isl -- the GEAK GEMM path passes it -- but the
forge path never did, so forge fell back to inferring one from the token
coverage list. That list is capped by conc (about 512 at conc=64), so a
long-context arm was described as a workload roughly 16x shorter than it is.

What that costs is not a missing top band: forge's ``conc * 128`` term reaches
the 8192 ceiling regardless. It is budget spent tuning mid bands (512, 2048)
the workload never prefills in, on a shape list forge then trims to fit the
window -- so the wasted entries can displace ones that matter.

Sent only when actually known. ``SharedState.isl`` defaults to 0, and at 0 the
option is omitted rather than sent as a zero, which keeps forge on its existing
inference and keeps the command accepted by forge builds that predate the
option. Pairs with the KernelForge side that adds ``--isl``; without it click
rejects the unknown option, so the two land together.

Co-authored-by: Cursor <cursoragent@cursor.com>

Bypass trace analysis can write a weighted, variant-discriminating
TraceShapeManifest, and forge has accepted --shapes-manifest since it was
added, but nothing ever passed it: the manifest was produced and dropped.

Resolve it from the latest trace analysis (the artifact_paths entry, the
trace_shape_manifest block, or beside the candidates file) and forward it.
Resolution is fail-open at every step -- a missing, unreadable, or
wrong-kind file yields "", leaving forge on demand, shapes JSON, untuned
CSV, or config derivation exactly as before.

The manifest stays behind two gates in practice: its production is opt-in
via HYPERLOOM_TRACE_SHAPE_MANIFEST, and forge short-circuits on demand
before reaching it, so a serving log carrying GEMM misses still wins.
This wires up the fallback rather than changing that precedence.

Co-authored-by: Cursor <cursoragent@cursor.com>

…e exists"

This reverts commit 660973d.

Wiring the manifest up bought almost nothing and cost three safety checks.

It buys almost nothing because forge short-circuits on demand before it ever
reaches the manifest, and the demand file is derived from the serving log
Hyperloom already passes -- so on any run whose log carries GEMM misses (the
case this whole line exists for) the manifest is never read. The bf16 dense
tuner does not read it at all.

The cost is that the resolution skipped guards its sibling shape sources
honour. It did not take require_fresh_profile, so a manifest from another
workload arm could be handed to the tuner; it sat before the framework != vllm
branch, making a trace manifest a shape source on the one framework that
branch exists to keep trace evidence out of; and it bypassed
_align_forge_shapes_for_aiter, so a freshly captured, workload-matched,
dispatch-aligned shape list could be silently replaced by an unaligned one --
which then tunes rows no runtime lookup reaches.

Hardening it would mean three additions guarding a path that is dead in
practice. Reverting is the smaller surface and the honest one. Forge keeps
accepting --shapes-manifest, so nothing has to change there if this comes back
with the guards and a reason to trust it over demand.

Co-authored-by: Cursor <cursoragent@cursor.com>

…ngth"

This reverts commit 31f6cd3.

Two reasons, either of which is enough on its own.

It breaks the whole forge GEMM lane on any deployment whose forge predates the
matching option. click rejects an unknown option and exits 2, forge never
prints its sentinel, and the run is recorded as a tuning failure with no
error_class -- indistinguishable from a crash while measuring. The trigger is
not an edge case: ISL comes from the workload env and defaults to 1024, so
isl > 0 holds on every real workload. The shared KernelForge checkout on the
serving box is on main at 6cdc7c4 and has no --isl (--shapes-manifest greps
11 hits there, --isl none), so this is the state a merge would land into, and
nothing here makes Hyperloom wait for the other half to arrive.

The option may also be the wrong shape to begin with. Prefill step M is the
chunked prefill size, not the input length, and Hyperloom does configure
--chunked-prefill-size. Where that setting is below ISL, declaring the ISL
collapses the mid bands (min(isl, 8192) folds them onto one) and leaves a row
at M=8192 that the runtime never looks up -- tuning wasted, by a different
route than a wrong key.

Sizing prefill bands from the workload is still worth doing. It needs the
value the scheduler actually batches, and a forge that accepts it.

Co-authored-by: Cursor <cursoragent@cursor.com>

Reading it back off the stack could name the wrong round's file.

_lift_to_current_best skips the stack append when (action, variant_name)
already matches, and a GEMM variant is named <backend>_<tuner>. So when a
second macro cycle re-tunes the same tuner and keeps it, nothing is appended
and the newest gemm_tuning entry still describes round one. Taking the artifact
from there had the second attempt claim the first one's path, and the breakdown
then credits it with the first one's gain -- the same misreport the backfill
was added to remove, pointing the other way.

The value never had to be looked up. The E2E loop already holds it when it
builds the stack entry: it is the candidate's env var, or the only value its
env carries. _candidate_tuned_file returns exactly that, and both the stack
entry and the attempt row take it from the same call, so they are the same
string by construction. _adopted_tuned_file is gone.

The earlier reasoning was half right: one KEEP really is described by three
different path strings (durable copy, tuner workspace, E2E merge product), so
re-deriving it does fail. The wrong step was concluding that the stack had to
be read -- the way out is to take it from where it is already known.

Verified by mutation, which also caught a hole in the test: dropping the
env_var preference left every case passing, because the expected path happened
to be the dict's first value and the fallback returned it anyway. With the
target key moved off the front, the mutation fails the case it should.
Controlled comparison on the four affected suites: 6 failed / 383 passed
before, 6 failed / 385 passed after, the same six Windows platform failures.

The false-claim path itself is covered in test_coordinator_gemm_promote_units,
which cannot be collected on Windows (recipe_kb imports fcntl), so that case
is exercised by Linux CI only.

Co-authored-by: Cursor <cursoragent@cursor.com>

The reason this lane exists is that MoE tuning keyed on the model config wrote
tables no runtime lookup could reach, so the key has to come from the dispatch
tuple the runtime logged. Both ends of that were covered -- the CSV writer in
test_gemm_bf16_aiter_routing, KernelForge's preference for a caller-supplied CSV
in test_fmoe_ck -- and the handoff between them was not covered at all.

Measured with mutations against the previous suite: setting the payload field to
"", removing the derivation from the log, removing the caller-CSV existence
check, and removing the argv option each left every test passing. The string
moe_untuned_csv did not appear in a single Hyperloom test payload. Deleting this
feature's plumbing outright was a green run.

Four handler-level cases now assert the chain: the derived CSV exists and its
fields equal the logged tuple rather than anything derivable from the config
(inter_dim sharded, both quant dtypes, quant type); the CSV the payload names is
the one written to the workspace; a caller-supplied CSV wins; and a path that no
longer exists falls back to deriving from the log instead of being forwarded
dead. Two tool-level cases assert the option reaches forge's argv, and one
asserts its absence when no key was observed.

test_build_cmd_maps_all_options also stops overstating itself. It claimed to map
all options while asserting 10 of the ones _build_cmd emits, which is how the
MoE option went unasserted while being the point of the lane -- it now checks
--untuned-csv, --kernel-signature-log, --tp, --conc and both timeouts too, and a
meta case fails when _build_cmd emits a flag this file does not declare, so the
next omission surfaces here.

All four mutations are now caught (4, 3, 1 and 1 failures respectively).
Controlled comparison: the 7 failures in the affected suites are identical
before and after, all of them the Windows-only fcntl / patch / path-separator
platform limits.

Co-authored-by: Cursor <cursoragent@cursor.com>

forge reports seven micro_decision wordings. The bridge to the coordinator
schema handled four, so partial_failure, empty_output and partial_output left
decision unset and status at "ok" -- which in the breakdown is exactly what a
genuine no_improvement looks like. Those three wordings exist to draw that
distinction, and the envelope was erasing it.

Two of them are worse than a lost label. partial_failure means one tuner
crashed while another delivered, and partial_output means a tuner wrote fewer
rows than it had shapes for -- the rows it wrote are deployable. Both arrive
with a recommended_env, and both were dropped on the floor: no decision, no
requires_e2e_validation, so a usable artifact was never measured. Bridging on
"delivered an env" rather than on the single word candidate is what those cases
needed.

The barren wordings now reach the envelope as an error_class. A crash, a tuner
that wrote zero rows, and a partial run whose survivors produced nothing are
three different outcomes, and none of them is an honest no_improvement -- which
stays deliberately unadorned, because the others are only legible against it.

Separately, a tuner that named its own failure was invisible above itself. The
jsonl audit row already lifted error_class out of tuners_run, with a comment
saying a crashed run and a barren one otherwise look alike; the breakdown and
the optimization stack read the envelope instead, where a run with every tuner
crashed arrived as status="failed" plus two empty strings. The same lift now
happens on the envelope, before the bridge, so a specific class outranks the
generic wording and the message travels with it.

Mutation-verified, five for five: re-gating the bridge on candidate fails 2
cases, removing the error_class for barren wordings fails 1, dropping either
tuner lift fails 1 each, and letting no_improvement acquire an error_class
fails 1. Controlled comparison across four suites: 8 failed / 422 passed before,
8 failed / 428 passed after, the same eight Windows platform limits.

Co-authored-by: Cursor <cursoragent@cursor.com>

Found while tracing the previous commit end to end. tuners_run is forge's own
JSON, so its shape is not guaranteed, and the two loops that lift an error out
of it iterated it directly: a scalar there raises TypeError, which the caller's
catch-all turns into "the tuning run failed" with a Python exception name for a
cause. That is precisely the misattribution this lane exists to remove, and the
previous commit introduced it while fixing a neighbouring instance of it.

Coerce a non-list to empty before either loop. The verdict still lands and the
cause stays honest.

Two cases added from the same trace. One pins the combination the change makes
reachable: partial_failure with an env and a crashed sibling now yields KEEP,
requires_e2e_validation, the crashed tuner's error_class, and a status that is
not "failed" -- promotability is decided on status, so a named crash must not
demote a run that delivered. The other feeds four malformed tuners_run shapes
(scalar, string, dict, list with non-dicts) and asserts a verdict still lands
with no exception class as the cause.

Also traced the downstream of the previous commit and found nothing else to
change. _should_run_bf16_dense_gemm_fallback rejects on micro_decision before it
ever reads extra_envs, so routing partial_* through the KEEP branch cannot
disturb it. _gemm_e2e_candidates filters on `status not in ("ok",
"partial_output")`, so the crashed tuner inside a partial_failure is excluded
while its delivering sibling becomes a candidate -- which is what makes routing
that wording to E2E correct rather than merely permissive. error_class has no
decision-making consumer on this path: the collective handler's check at 5146 is
a different lane, and promotability keys on status.

Co-authored-by: Cursor <cursoragent@cursor.com>

… KEEP

Two defects found reviewing this branch, both on the MoE path this lane exists
to fix, and both invisible to the tests because the tests disagreed with the
caller.

The token column was wrong or fatal. _write_fmoe_untuned_csv_from_log annotated
tokens as list[int] and iterated it, but its only production caller builds them
with _normalize_tokens, which returns forge's comma-separated string. A real
multi-token workload reached int(',') and lost the whole MoE tuning to a
ValueError the envelope then reported as a forge crash; a single-token workload
silently wrote each digit as its own token, producing rows no runtime lookup can
reach. Measured: '1,32,64' raises, '64' yields tokens 4 and 6. Every existing
case passed a list, so the suite agreed with the annotation rather than with the
caller. _fmoe_token_list now accepts either shape and drops unparseable or
non-positive entries instead of raising -- this is a token sweep for a tuning
input, and one bad entry is not worth the run.

Coverage treated the token count as part of a problem's identity. The tuner
sweeps token and emits one row per batch size it chose; the runtime asks for
whichever batch size it is running. Requiring them to be equal made a table that
does serve the problem report zero coverage, and a zero there goes into
apply_blockers and vetoes a KEEP whose throughput really improved -- exactly the
misjudgement this module was added to prevent, reproduced on the MoE path. The
two constants in this change disagreed with each other about it:
_FMOE_SHAPE_FIELDS omits token with the comment "which the tuner sweeps", while
_FMOE_DISPATCH_COLUMNS included it "because the tuner emits one row per swept
batch size" -- the same fact, read as the opposite conclusion. Identity is now
_FMOE_PROBLEM_COLUMNS; _FMOE_DISPATCH_COLUMNS keeps its job of locating and
validating a row's fields.

Mutation-verified, four for four: iterating tokens as a list, dropping the string
branch, and keeping non-positive tokens each fail a case, and putting token back
into the identity fails three. Controlled comparison across five suites: 8 failed
/ 465 passed before, 8 failed / 468 passed after, the same eight Windows platform
limits.

Co-authored-by: Cursor <cursoragent@cursor.com>

…ead branch

Two review findings, one of them mine.

The guard around _validate_gemm_tuning_e2e recorded the fault and left the
envelope alone. But the forge bridge had already stamped decision="KEEP",
requires_e2e_validation=True and the raw combined recommended_env on the strength
of the micro result, and the normal exit of validation rewrites all three
precisely so Orchestration never sees an unmeasured candidate and issues a
bundled integrate against it. An arm that raised was not measured, so it now
reads as REVERT with the envs cleared and micro_decision naming the exception.
The fault record stays: the point is that the reason is legible, not that the run
looks clean.

The other finding is a branch I added a few commits ago that cannot execute.
_FORGE_DELIVERING_MICRO_DECISIONS listed partial_failure and partial_output on the
theory that either can arrive with a deployable env. forge's build_report checks
has_candidate ahead of both, so any run that produced an env reports "candidate"
instead -- verified against the real build_report across seven scenarios,
including "one tuner crashed while another delivered", which reports candidate,
not partial_failure. The branch was unreachable, its comment described a scenario
that cannot occur, and two tests asserted an input forge never emits while
passing. Reverted to gating on candidate; the wordings still get their verdict
and their error_class through the barren branch, which is where they actually
arrive. The surviving case now pins the real contract, and the crashed-sibling
case is expressed as what forge really sends.

Worth recording: those two tests were the same defect I had flagged in someone
else's fixture hours earlier -- an assertion whose input the producer cannot
generate. Passing tests were the reason it went unnoticed.

Mutation-verified: dropping the decision reset, keeping requires_e2e_validation,
or leaving the stale envs each fail a case. Suites: test_gemm_shape_coverage 38
passed, the bridge cases 7 passed.

Co-authored-by: Cursor <cursoragent@cursor.com>

The comments I wrote around these fixes were carrying the commit messages.
_fmoe_token_list had an 11-line docstring over 18 lines of code, most of it
retelling the defect; _FORGE_BARREN_MICRO_DECISIONS spent 6 of 11 lines on how
the contract was verified; several test docstrings ran 6-9 lines explaining
consequences already argued in the commit that introduced them.

Each now states the constraint a reader cannot get from the code -- that
build_report checks has_candidate first, that token is swept by the tuner and
therefore not an identity, that validate is inside the guard because it now runs
int() over raw config -- and nothing else. No behaviour change; suites and lint
unchanged.

Co-authored-by: Cursor <cursoragent@cursor.com>

Mutating the nogit branch of the revert trigger and of
_revert_warm_patch_state left the whole warm suite green, so widening the
trigger to (pre_sha or nogit_backups) was unguarded: a nogit apply has no
sha, and skipping its revert leaks the patch into later tasks that reuse
the same checkout. Cover both ends -- apply records the backups, revert
restores from them.

The apply-side case needs the patch CLI and skips on Windows; the
apply/revert round-trip was verified against a real POSIX patch on Linux
(modify, create, multi-file; tree byte-identical after revert).

Co-authored-by: Cursor <cursoragent@cursor.com>

main grew its own fused-MoE coverage check (PR #1245) while this branch
carried an independent one. git merged both happily because they sit at
different offsets in the same module, which is worse than a conflict: our
fmoe_tuned_config_coverage and _normalize_fmoe_field were defined second and
silently shadowed main's, dropping its kernelName check, q_dtype/act_type
normalisation and merged-CSV resolution with nothing to show for it.

main's implementation wins. Removed our _FMOE_DISPATCH_COLUMNS,
_FMOE_PROBLEM_COLUMNS, fmoe_dispatch_key, tuned_fmoe_csv_keys, the duplicate
_normalize_fmoe_field and the duplicate fmoe_tuned_config_coverage, and routed
fmoe_ck to main's _fmoe_tuned_config_coverage from inside
_gemm_tuned_config_coverage_impl -- so it also inherits our wrapper's
degrade-to-undetermined guard.

Dropped our token-agnostic matching. Real dispatch logs plus aiter's own
lookup settle it against us: aiter normalises token to nextPow2 and only
falls back across tiers above 32768, so token really is part of the key.
Of 15 logged dispatches, 7 (token 1, 2 and 1024-16384) have no CSV row and
genuinely ran default -- main's strict key reports that honestly, and
excluding token would have hidden it.

Kept what main does not have: the coverage wrapper's exception degradation,
the dense fail-open when a CSV yields no keys (main still reports 0% there
and lets an unreadable artifact veto a KEEP), and the MoE runtime-key
extraction feeding --moe-untuned-csv. Retargeted the fail-open tests at the
dense path they now guard.

Co-authored-by: Cursor <cursoragent@cursor.com>

Both nogit tests I added asserted result["status"], but only the
required-timeline path returns a dict; the legacy path has returned the
applied list since #808. On Linux both raised TypeError.

They passed locally only because they skip without a patch CLI, and this box
had none on PATH -- so the suite was green here and red in CI. Git for
Windows ships patch.exe under usr/bin; with that on PATH the file runs 33
passed / 1 skipped instead of 31 / 3, and these two now actually execute.

Co-authored-by: Cursor <cursoragent@cursor.com>